[CS2113-T12-3] Student Exchange Programme Helper by L0Z1K · Pull Request #22 · nus-cs2113-AY2223S2/tp

L0Z1K · 2023-03-02T04:23:14Z

Preparing NUS Mech Eng students to embark on a SEP, as journeying into a foreign country is overwhelming. Our solution will be a quick way to let them know which modules can be mapped to their desired university and the better flight and accommodation options that are within their budget.

Jeraldchen · 2023-03-29T05:13:56Z

+Sequence Diagram of Storage initialisation:
+
+![Storage.png](diagrams%2FStorage%2FStorage.png)
+


Image looks blur on the website

Jeraldchen · 2023-03-29T05:14:53Z

+# Architecture
+
+![Architecture.png](diagrams%2FArchitecture.png)
+


Architecture is clear and easy to understand.

Jeraldchen · 2023-03-29T05:17:20Z

+The following sequence diagram shows the relationship between the classes involved when the delete command is called.
+
+![HelpCommandSequenceDiagram.png](diagrams%2FHelpCommandSequenceDiagram.png)
+


Activation bar of :HelpCommandModule appears to be cut off from the top and bottom

Jeraldchen

Overall DG is clear and well done. Just some minor errors to look into as shown in comments

wanjuin

goos use of sequence diagram (UML) in the DG overall.

BenjaminPoh

Overall good job, explanations were clear and diagrams were easy to understand!

BenjaminPoh · 2023-03-30T01:08:57Z

+2. Parser : Processes and Executes User Commands
+3. UI : Prints out messages to user
+4. Storage : Processes and stores
+5. DataReader


Is there a missing whitespace here? (Diagram says "Data Reader")

BenjaminPoh · 2023-03-30T01:38:16Z

+
+The following class diagrams illustrates the relationship between the Parser class and the Command classes.
+- (TODO: finish up the rest of the command cases)
+![ParserSequenceDiagram.png](diagrams%2FParserSequenceDiagram.png)


The UML Diagram is very large, and many of the command classes are still incomplete. Maybe try to break it up into different diagrams or use reference frames

There is also a case of 4-level self-invocation. Maybe can also make use of reference frames for this?

BenjaminPoh · 2023-03-30T01:44:00Z

+
+The following sequence diagram shows the relationship between the classes involved when the delete command is called.
+
+![DeleteModuleCommandSequenceDiagram.png](diagrams%2FDeleteModuleCommandSequenceDiagram.png)


For return values, I think it is sufficient to write "true" instead of "return true", as it should already be clear it is a return value by the dotted arrow.

BenjaminPoh · 2023-03-30T01:48:48Z

+
+Sequence Diagram of List Current Command.
+
+![ListCurrentCommandSequenceDiagram.png](diagrams%2FListCurrentCommandSequenceDiagram.png)


Is there a reason why the dotted arrow at the end of ui:UI goes leftwards instead of right like other arrows?

euzhengxi · 2023-03-30T07:13:38Z

 ## Acknowledgements

-{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
+{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the


missing information?

euzhengxi · 2023-03-30T07:14:46Z

+DataReader class reads two external .txt files to acquire the list of Partner Universities and list
+of Modules available in the PUs, and provides this information to other components.
+
 {Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}


the uml diagram is missing

euzhengxi · 2023-03-30T07:16:08Z

+Sequence Diagram of Storage initialisation:
+
+![Storage.png](diagrams%2FStorage%2FStorage.png)
+


euzhengxi · 2023-03-30T07:21:23Z

+
+Sequence Diagram of List Current Command.
+
+![ListCurrentCommandSequenceDiagram.png](diagrams%2FListCurrentCommandSequenceDiagram.png)


arrow is on the wrong side

euzhengxi · 2023-03-30T07:23:57Z

+> Syntax: list current [_uniAbbreviation_]
+
+Sequence Diagram of List Current Pu Command
+![ListCurrentPuCommandSequenceDiagram.png](diagrams%2FListCurrentPuCommandSequenceDiagram.png)


missing activation bars

euzhengxi · 2023-03-30T07:34:56Z

+
+The following sequence diagram shows the relationship between the classes involved when the delete command is called.
+
+![DeleteModuleCommandSequenceDiagram.png](diagrams%2FDeleteModuleCommandSequenceDiagram.png)


activation bar should end when the method ends

dsicol · 2023-03-30T07:23:39Z

+The help command provides a list of commands and the commands' respective input format for the user.  
+> Syntax: /help
+
+The following sequence diagram shows the relationship between the classes involved when the delete command is called.


There is a typo here regarding the delete command

dsicol · 2023-03-30T07:29:11Z

+
+The following sequence diagram shows the relationship between the classes involved when the delete command is called.
+
+![HelpCommandSequenceDiagram.png](diagrams%2FHelpCommandSequenceDiagram.png)


The method call printHelpCommandMessage() from :HelpModuleCommand should be at the start of the activation bar.

dsicol · 2023-03-30T07:34:06Z

+**Note: Partner Universities Abbreviations can be found using List Pu command**
+
+Sequence Diagram of List Pu Modules Command.
+![ListPuModulesCommandSequenceDiagram.png](diagrams%2FListPuModulesCommandSequenceDiagram.png)


Nice diagram! But the final return from the static UI class should be at the end of the activation bar

dsicol · 2023-03-30T07:36:28Z

+
+Sequence Diagram of List Current Command.
+
+![ListCurrentCommandSequenceDiagram.png](diagrams%2FListCurrentCommandSequenceDiagram.png)


Nice! It would be better if the self-invoked method had an return arrow on the same side of the activation bar going back into the bar.

dsicol · 2023-03-30T07:38:54Z

+
+Sequence Diagram of Storage initialisation:
+
+![Storage.png](diagrams%2FStorage%2FStorage.png)


It would be better to add figure numbers to your diagrams too!

itszhixuan

Generally well done, except for some small mistakes.

itszhixuan · 2023-03-30T07:21:18Z

+Sequence Diagram of Storage initialisation:
+
+![Storage.png](diagrams%2FStorage%2FStorage.png)


Sequence diagram looks blur, but it seems like the return values are not indicated with a dotted line.

itszhixuan · 2023-03-30T07:26:17Z

+The commands that the parser class will initialise are ListPuCommand(), ListCurrentCommand(modules),
+prepareListPuModulesCommand(userCommandSecondKeyword, universities), ExitCommand(),
+prepareAddModuleCommand(storage, userCommandSecondKeyword, puModules, universities),
+DeleteModuleCommand(storage, indexToRemove, modules), and HelpCommand(). The parser class will handle error checking by


Perhaps you can consider reformatting this to make it easier to read, as all the commands are lumped together and messy.

itszhixuan · 2023-03-30T07:32:34Z

+
+The following class diagrams illustrates the relationship between the Parser class and the Command classes.
+- (TODO: finish up the rest of the command cases)
+![ParserSequenceDiagram.png](diagrams%2FParserSequenceDiagram.png)


The third alt case does not state the condition explicitly, which makes it unclear on when this is called.

itszhixuan · 2023-03-30T07:42:00Z

+
+### Add Module Command
+
+Adds the Module the user has wants to save to the saved modules database.


Is this line correct? I am still able to understand but seems like there is some typo.

itszhixuan · 2023-03-30T07:45:02Z

+> Syntax: list current [_uniAbbreviation_]
+
+Sequence Diagram of List Current Pu Command
+![ListCurrentPuCommandSequenceDiagram.png](diagrams%2FListCurrentPuCommandSequenceDiagram.png)


Another small typo in the image "UserConcole"

TopGun2001 · 2023-03-31T02:43:26Z

+**Note: Partner Universities Abbreviations can be found using List Pu command**
+
+Sequence Diagram of List Pu Modules Command.
+![ListPuModulesCommandSequenceDiagram.png](diagrams%2FListPuModulesCommandSequenceDiagram.png)


I feel like your UML diagram is a bit big and filled with a lot of words. Maybe you could try to make it more concise by making it small and precise.

…into Branch-DeadlineStorage-Singleton # Conflicts: # src/main/java/seedu/duke/Duke.java # src/test/java/seedu/duke/ParserTest.java # src/test/java/seedu/duke/command/DeleteModuleCommandTest.java # src/test/java/seedu/duke/command/ExitCommandTest.java # src/test/java/seedu/duke/command/HelpCommandTest.java # src/test/java/seedu/duke/command/InvalidCommandTest.java

Change DeadlineStorage classs into singleton pattern

…into branch-Parser-Singleton # Conflicts: # src/test/java/seedu/duke/command/DeleteModuleCommandTest.java # src/test/java/seedu/duke/command/ExitCommandTest.java # src/test/java/seedu/duke/command/HelpCommandTest.java # src/test/java/seedu/duke/command/InvalidCommandTest.java

Change Parser class into singleton pattern

Update DG Deadline and Budget test details

Fix bugs in DG

Update parser sequence diagrams

…outcomes

Update PPP

Update DG

Update AboutUs

Update Developer Guide Budget Commands

Update PPP

Format budget DG

Change UG DG QUICK START

Jeraldchen reviewed Mar 29, 2023

View reviewed changes

wanjuin reviewed Mar 29, 2023

View reviewed changes

BenjaminPoh reviewed Mar 30, 2023

View reviewed changes

euzhengxi reviewed Mar 30, 2023

View reviewed changes

dsicol reviewed Mar 30, 2023

View reviewed changes

itszhixuan reviewed Mar 30, 2023

View reviewed changes

TopGun2001 reviewed Mar 31, 2023

View reviewed changes

briantjs00 and others added 20 commits April 6, 2023 00:51

Update to Parser class to make it more singleton

9318f2c

Change BudgetPlanner class into singleton pattern

e6d611a

Update the instantiation of BudgetPlanner in main and related tests

46f67b1

Change BudgetStorage class into singleton pattern

f0750c1

Update the instantiation of BudgetPlanner in main and related tests.

11b5e6f

Change Storage class into singleton pattern

307ef54

Update the instantiation of Storage in related classes and tests

6615696

Change DeadlineStorage class into singleton pattern

affd10e

Update the instantiation of DeadlineStorage in related classes and tests

2af669b

Merge pull request #157 from briantjs00/branch-Budget-Singleton

1218936

Merge pull request #158 from briantjs00/branch-Storage-Singleton

9f1d7a7

Add sequence diagram png

20fd061

Add Developer Guide for budget class

477c965

Merge branch 'master' into branch-add-budget-developer-guide

457d2b6

Merge Conflicts

168c842

Merge pull request #159 from briantjs00/Branch-DeadlineStorage-Singleton

2bedfc5

Change DeadlineStorage classs into singleton pattern

Move the instantiation of Paser to Duke main

bb60332

Merge pull request #156 from briantjs00/branch-Parser-Singleton

62cb629

Change Parser class into singleton pattern

SSzeWen and others added 30 commits April 10, 2023 21:02

Update Budget Diagrams in Parser

c4912b6

Add Developer Guide details

8602f6c

Merge branch 'master' into branch-add-highlevel

d3e6217

Merge pull request #214 from SSzeWen/branch-add-highlevel

a45026b

Update DG Deadline and Budget test details

Fix bugs in DG

249c30d

Merge pull request #215 from darrenlsx/branch-fix-bugs-6

789451b

Fix bugs in DG

Update Developer Guide Budget Commands

d36bf98

Update Parser Sequence Diagrams

ab21883

Update DG

d4190be

Merge pull request #217 from briantjs00/branch-Update-Parser-Diagrams

6a2e044

Update parser sequence diagrams

Delete dummy PPP

eacae8d

Update PPP

0972fed

Add User Guide notes and change expected outcomes and description of …

ea768ce

…outcomes

Merge branch 'master' into branch-touchup

a181c51

Merge pull request #218 from MuxPotato/FinalisePPP

6904d3e

Update PPP

Fixed Typo

18e01b6

Update DG

34e80f2

Update DG for searchByNusModCode

1e63acc

Merge pull request #219 from briantjs00/branch-Update-PPP

b286f5b

Update DG

Add in Jar file link

1113acd

Update AboutUs

0974893

Merge pull request #220 from MuxPotato/UpdateAboutUsForWebsite

e31c97c

Update AboutUs

Merge branch 'master' into branch-touchup

5ed231f

Merge pull request #216 from SSzeWen/branch-touchup

05dcb54

Update Developer Guide Budget Commands

Update PPP

025f222

Merge pull request #221 from briantjs00/branch-Update-PPP

67985de

Update PPP

Format budget DG

2335648

Merge pull request #222 from SSzeWen/branch-final-fix

400e17f

Format budget DG

Change UG DG QUICK START

9b2de8e

Merge pull request #223 from MuxPotato/LastChangeToUGDG

f8487bb

Change UG DG QUICK START

		Sequence Diagram of Storage initialisation:

		![Storage.png](diagrams%2FStorage%2FStorage.png)

		# Architecture

		![Architecture.png](diagrams%2FArchitecture.png)

		The following sequence diagram shows the relationship between the classes involved when the delete command is called.

		![HelpCommandSequenceDiagram.png](diagrams%2FHelpCommandSequenceDiagram.png)


		The following sequence diagram shows the relationship between the classes involved when the delete command is called.

		![DeleteModuleCommandSequenceDiagram.png](diagrams%2FDeleteModuleCommandSequenceDiagram.png)


		Sequence Diagram of List Current Command.

		![ListCurrentCommandSequenceDiagram.png](diagrams%2FListCurrentCommandSequenceDiagram.png)


		### Add Module Command

		Adds the Module the user has wants to save to the saved modules database.

Conversation

L0Z1K commented Mar 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jeraldchen left a comment

Choose a reason for hiding this comment

Uh oh!

wanjuin left a comment

Choose a reason for hiding this comment

Uh oh!

BenjaminPoh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itszhixuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants